Skip to content

Standardize CREATE TABLE options equals signs #1751

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mvzink
Copy link
Contributor

@mvzink mvzink commented Feb 27, 2025

  • Make spaces around equals signs canonical: ENGINE = InnoDB instead of ENGINE=InnoDB
  • Make equals signs canonical: AUTO_INCREMENT = 100 instead of AUTO_INCREMENT 100
  • Make equals signs optional for charset, engine, and collation to match MySQl: COLLATE utf8mb4_unicode_ci works as well as COLLATE = utf8mb4_unicode_ci

In some dialects, some options might require the equals signs, so will be more permissive in some situations. For example, ClickHouse requires the equals signs for ENGINE. It didn't seem worth special casing this to me.

* Make spaces around equals signs canonical: `ENGINE = InnoDB` instead of `ENGINE=InnoDB`
* Make equals signs canonical: `AUTO_INCREMENT = 100` instead of `AUTO_INCREMENT 100`
* Make equals signs optional for charset, engine, and collation to match MySQl: `COLLATE utf8mb4_unicode_ci` works as well as `COLLATE = utf8mb4_unicode_ci`

In some dialects, some options might require the equals signs, so will be more permissive in some situations. For example, [ClickHouse] requires the equals signs for `ENGINE`. It didn't seem worth special casing this to me.

[ClickHouse]: https://clickhouse.com/docs/en/sql-reference/statements/create/table/#create-table-options
@mvzink
Copy link
Contributor Author

mvzink commented Feb 27, 2025

Note that a version of #1747 which standardizes all options a la SqlOption could supersede this.

}

#[test]
#[ignore = "not yet supported"]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Happy to remove this, especially if it ends up being covered by #1747 or something.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I think we can remove and add it back when we have support for the syntax

Comment on lines +1058 to +1059
"CREATE TABLE foo (id INT) COLLATE utf8mb4_unicode_ci",
"CREATE TABLE foo (id INT) COLLATE = utf8mb4_unicode_ci",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm thinking the mismatches aren't ideal if we would rather represent the input SQL faithfully?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, I think ideally these options would all be in a vector similar to Vec<SqlOption>, but with a flag for the equals sign like I added for ALGORITHM and AUTO_INCREMENT, which would avoid these mismatches. Maybe I will see how #1747 shakes out and then revisit this, and mark this draft in the meantime.

}

#[test]
#[ignore = "not yet supported"]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I think we can remove and add it back when we have support for the syntax

@mvzink mvzink marked this pull request as draft February 28, 2025 17:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants